feat: add a default home page #16

raymondfeng · 2018-09-24T15:51:29Z

Add a default home page for the app. We'll use it as a template for loopbackio/loopback-next#1745.

We need to fix @loopback/rest so that it respects the correct content-type from responses. At the moment, the home page will render raw html as the media type is plain/text for string response from the controller method.

virkt25 · 2018-09-24T16:08:49Z

I made the following changes to get the page to render :D

Render:

Looks good overall -- have some concerns with the resources we're loading -- will leave a comment.

virkt25 · 2018-09-24T16:11:41Z

src/controllers/home-page.controller.ts

+  <meta name="viewport" content="width=device-width, initial-scale=1">
+
+  <!-- Latest compiled and minified CSS -->
+  <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/css/bootstrap.min.css" integrity="sha384-BVYiiSIFeK1dGmJRAkycuHAHRg32OmUcww7on3RYdg4Va+PmSTsz/K68vbdEjh4u"


Can we skip out on loading the full bootstrap.min.css and the accompanying js file? We're using about 2 styles from this entire file. I'd rather we copy the style in-lined for a faster render. No JS is needed. (Currently this + JS is loading 63.1 KB while the page payload is only 1.7 KB).

An alternative IIRC is that BootStrap also provides just the grid CSS -- which is lighter and includes the CSS for container class.

virkt25 · 2018-09-24T16:12:09Z

src/controllers/home-page.controller.ts

+  </footer>
+  <!-- Bootstrap core JavaScript -->
+  <!-- Placed at the end of the document so the pages load faster -->
+  <script src="https://ajax.googleapis.com/ajax/libs/jquery/1.12.4/jquery.min.js"></script>


Nothing on this page needs the JS ... no BootStrap widgets are used.

virkt25 · 2018-09-24T16:12:50Z

src/controllers/home-page.controller.ts

+  <div class="container">
+    <h1>${this.name}@${this.version}</h1>
+    <ul>
+      <li><a href="/openapi.json">OpenAPI spec: /openapi.json</a></li>


Can we style the list to be bigger. Plus can OpenAPI spec: not be part of the link. Bolded. Would be a better style imo.

Same applies for the bullet below.

virkt25 · 2018-09-24T16:14:11Z

src/controllers/home-page.controller.ts

+  </div>
+  <footer>
+    <div class="container">
+      <p>


Let's center this text on the page.

raymondfeng · 2018-09-24T16:56:35Z

@virkt25 You worked around the issue by manipulating Express response directly. In this case, we should introduce a special return value such as ByPassResponse so that our writer won't try to send 204.

Feel free to improve the UI. I'm a beginner there :-).

virkt25 · 2018-09-24T17:00:39Z

You worked around the issue by manipulating Express response directly. In this case, we should introduce a special return value such as ByPassResponse so that our writer won't try to send 204.

I think a simpler fix can even be just to check if Content-Type header has been set in the writer ... and if so we don't overwrite it -- and then we can rely on the response.end in the write.

And of course in the long term it should request the responses decorator + the expected content type header.

Feel free to improve the UI. I'm a beginner there :-).

I'll push some changes to the PR in a bit :)

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng · 2018-09-24T21:39:33Z

Please note that the CI failure is not related to this change. It will be fixed as part of #14.

virkt25 · 2018-09-24T23:16:51Z

Here's a screenshot for the UI -- no external CSS / JS. Minor variances with some alternatives below. I personally vote for Alternative 1 (I tried it after pushing my changes otherwise I would've included that in the PR). Anyone else have any preferences? cc: @raymondfeng @bajtos @strongloop/sq-lb-apex

Current:

Alternative 1:

Alternative 2:

Alternative 3:

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

bajtos · 2018-09-25T08:55:52Z

I vote for Alternative 1.

Perhaps instead of using name@version format, can we move the version number to line 2, to make it more explicit what is version and what is the app name?

A mock-up:

@loopback/example-shopping
version 1.0.0

bajtos · 2018-09-25T09:00:04Z

public/index.html.template

+<body>
+  <h1 style="text-align: center">${name}@${version}</h1>
+  <h3>OpenAPI spec: <a href="/openapi.json">/openapi.json</a></h4>
+  <h3>API Explorer: <a href="/explorer">/explorer</a></h4>


IIRC, applications can customize the URL of both OpenAPI spec and Explorer. See loopbackio/loopback-next#1637, Customize How OpenAPI Spec is Served and Configure the API Explorer.

IMOL, the home page should respect these settings. Eventually :) I am fine to keep the current static version as the first baby step.

bajtos · 2018-09-25T09:00:54Z

src/controllers/home-page.controller.ts

+  description: string;
+  render: TemplateExecutor;
+
+  constructor(@inject(RestBindings.Http.RESPONSE) private res: Response) {


Can we use response instead of res please?

bajtos · 2018-09-25T10:34:45Z

src/controllers/home-page.controller.ts

+import {inject} from '@loopback/context';
+import {RestBindings, Response} from '@loopback/rest';
+
+const pkg = require('../../../package.json');


From the perspective of clean design and Dependency Injection pattern, I think controller should not depend package.json directly, but get it injected from app context. Then the application should bind the content of package.json (maybe a subset of properties only?) in its constructor.

Not a big deal for this particular pull request, but I think we should think more about the design patterns when we start working on adding HomePageController to our project template.

Yes, I'm thinking about the same direction while I work on this implementation.

Signed-off-by: Raymond Feng <[email protected]>

Signed-off-by: virkt25 <[email protected]>

Signed-off-by: Raymond Feng <[email protected]>

raymondfeng requested review from bajtos and virkt25 September 24, 2018 15:51

raymondfeng force-pushed the home-page branch from f37bc9f to 0f3779c Compare September 24, 2018 15:52

virkt25 reviewed Sep 24, 2018

View reviewed changes

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

7af3258

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng mentioned this pull request Sep 24, 2018

[WIP] feat(rest): allow bypassing http response writing and custom content type loopbackio/loopback-next#1753

Closed

7 tasks

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

d986a61

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng mentioned this pull request Sep 24, 2018

Scaffolded LB4 app reports 404 for the home page loopbackio/loopback-next#1745

Closed

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

e8bbf7b

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng force-pushed the home-page branch 2 times, most recently from 4667561 to 2258e98 Compare September 24, 2018 18:28

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

61099ca

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

df2a5fa

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

21c4f41

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 24, 2018

feat(rest): allow bypassing http response writing and custom content …

b4808b0

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

raymondfeng force-pushed the home-page branch 2 times, most recently from f1cc2bb to 23da6cb Compare September 24, 2018 22:34

raymondfeng added a commit to loopbackio/loopback-next that referenced this pull request Sep 25, 2018

feat(rest): allow bypassing http response writing and custom content …

deb2f05

…type We discover an issue to return html from a controller: See loopbackio/loopback4-example-shopping#16

bajtos approved these changes Sep 25, 2018

View reviewed changes

bajtos reviewed Sep 25, 2018

View reviewed changes

raymondfeng added 3 commits September 25, 2018 08:07

feat: add a default home page

d00cd44

Signed-off-by: Raymond Feng <[email protected]>

chore: move html template to /public

0a5a046

Signed-off-by: Raymond Feng <[email protected]>

fix: add response object for ping

e48f215

Signed-off-by: Raymond Feng <[email protected]>

virkt25 and others added 2 commits September 25, 2018 08:09

feat: update index.html design

7197f17

Signed-off-by: virkt25 <[email protected]>

chore: clean up ping controller

af2db9d

Signed-off-by: Raymond Feng <[email protected]>

raymondfeng force-pushed the home-page branch 2 times, most recently from 7cfc221 to 226347d Compare September 25, 2018 15:10

fix: improve home page controller with di of pkg

f0c76eb

Signed-off-by: Raymond Feng <[email protected]>

raymondfeng force-pushed the home-page branch 2 times, most recently from 366433c to 5334669 Compare September 25, 2018 18:09

chore: simplify test cases

df3c287

Signed-off-by: Raymond Feng <[email protected]>

raymondfeng force-pushed the home-page branch from 5334669 to df3c287 Compare September 25, 2018 18:11

raymondfeng mentioned this pull request Sep 25, 2018

user order history component #12

Merged

raymondfeng merged commit 293177b into master Sep 25, 2018

raymondfeng deleted the home-page branch September 25, 2018 18:19

raymondfeng mentioned this pull request Sep 26, 2018

feat(boot): bind content of package.json to app context loopbackio/loopback-next#1764

Merged

7 tasks

bajtos mentioned this pull request Oct 1, 2018

feat(cli): add code template for default home page controller loopbackio/loopback-next#1763

Merged

7 tasks

bajtos mentioned this pull request Mar 19, 2019

File downloads (streaming responses) loopbackio/loopback-next#2230

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add a default home page #16

feat: add a default home page #16

raymondfeng commented Sep 24, 2018

virkt25 commented Sep 24, 2018

virkt25 Sep 24, 2018

virkt25 Sep 24, 2018

virkt25 Sep 24, 2018

virkt25 Sep 24, 2018

raymondfeng commented Sep 24, 2018

virkt25 commented Sep 24, 2018 •

edited

Loading

raymondfeng commented Sep 24, 2018 •

edited

Loading

virkt25 commented Sep 24, 2018

bajtos commented Sep 25, 2018

bajtos Sep 25, 2018

bajtos Sep 25, 2018

bajtos Sep 25, 2018

raymondfeng Sep 25, 2018

feat: add a default home page #16

feat: add a default home page #16

Conversation

raymondfeng commented Sep 24, 2018

virkt25 commented Sep 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raymondfeng commented Sep 24, 2018

virkt25 commented Sep 24, 2018 • edited Loading

raymondfeng commented Sep 24, 2018 • edited Loading

virkt25 commented Sep 24, 2018

Current:

Alternative 1:

Alternative 2:

Alternative 3:

bajtos commented Sep 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

virkt25 commented Sep 24, 2018 •

edited

Loading

raymondfeng commented Sep 24, 2018 •

edited

Loading